fix: scope circuit device ids to the panel that owns them - #22
Merged
Conversation
`stable_circuit_uuid` hashed the YAML circuit `id` alone, and those ids are shared vocabulary across the shipped configs — `default_MAIN_32.yaml` and `default_MAIN_40.yaml` both define a `solar_inverter` and an `oven`. One panel per broker makes that harmless, which is why it survived. Two panels in one add-on share a broker: the two `solar_inverter`s claim the same Homie device id and the same `ebus/5/<uuid>/...` topics, both write every tick, and a consumer reads each circuit's meters as the two unrelated loads alternating. Home Assistant's dip compensation then fires continuously on counters that fall as often as they rise. The name hashed is now `<panel-serial>/<circuit-id>`, which scopes a circuit the way every other id here is already scoped to its proxier. The separator is `/` because a panel serial may contain `-`, and any separator the serial can contain admits two different (panel, circuit) pairs hashing the same name. The derivation is shared byte-for-byte with the flat simulator, which publishes the same circuit ids for the same config — the firmware-upgrade rehearsal stops one and starts the other on one panel. `tests/test_instance_ids_contract.py` pins the value rather than the formula so a one-sided change fails CI. Real firmware cannot hit the collision, since a panel has its own broker.
Circuit device ids are now panel-scoped, so a simulated panel already added to Home Assistant has to be removed and re-added.
One pinned value cannot distinguish the panel scoping from any other change that happens to agree on a single input. `sim-40t-002` differs from `sim-40t-001` only in the serial, so the pair pins the serial as an input rather than as a constant folded into the namespace. Both literals are the ones the simulator's suite asserts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The collision
stable_circuit_uuidderived a circuit's Homie device id from the YAML circuitidalone. Those ids are shared vocabulary across the shipped configs —default_MAIN_32.yamlanddefault_MAIN_40.yamlboth define asolar_inverterand anoven. A real panel has its own broker, so it cannot collide; two simulated panels in one add-on share one, and the twosolar_inverters then claim the same device id and the sameebus/5/<uuid>/...topics.Both panels write every tick, so a consumer reads each circuit's meters as the two unrelated loads alternating. Observed on ha-test with
sim-32t-001andsim-40t-001: 16 energy counters flipping between the two panels' values every tick (space 12 exported-energy 53.18 → 37.44 → 53.27 → 37.52), with Home Assistant's dip compensation firing continuously.The change
stable_circuit_uuid(panel_id, circuit_id)hashes<panel-serial>/<circuit-id>, scoping a circuit the way every other id ininstance_idsis already scoped to its proxier. The namespace is unchanged. The separator is/and not-because a panel serial may itself contain-, and any separator the serial can contain admits two different (panel, circuit) pairs hashing the same name.Every caller now threads the panel serial from
panel_config.serial_number— the one key, never a second one alongside it._feeds_for_device_typereads it from the config it is already walking rather than taking it as a parameter, so a feed id cannot be built from a serial that does not own those circuits.Cross-repo contract
The flat simulator publishes the same circuit ids for the same config, and the firmware-upgrade rehearsal stops one and starts the other on a single panel — a circuit whose id changed at the swap strands its Home Assistant history. The identical change lands in
SpanPanel/simulator#50.tests/test_instance_ids_contract.pypins the derivation as values rather than as a formula:The simulator's suite asserts the same two, so a change made on one side alone fails CI on both. Two rather than one because a single value cannot distinguish the panel scoping from any other change that happens to agree on one input; the pair differs only in the serial, which pins the serial as an input rather than as a constant folded into the namespace. A third test states the bug itself: two panels sharing a circuit id must not share a device id.
Fixtures
golden_wire.jsonwas recaptured withscripts/capture-wire.py.golden_tree.jsonhad its circuit ids rewritten in place, andgolden_report.jsonwas regenerated from it perDEVELOPER.md— the regenerated report is the old one's observations unchanged as a multiset, reordered only because the report sorts by device id.Upgrade note
Released as 2.5.3. A simulated panel already added to Home Assistant has to be removed and re-added: its circuits get new device ids, so its circuit entities get new unique ids.
Full suite (394 tests) and
pre-commit run --all-filesare green.